Skip to content

Fixed interpretation calls within DML #1411

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Apr 23, 2025
Merged

Fixed interpretation calls within DML #1411

merged 1 commit into from
Apr 23, 2025

Conversation

@Hydrocharged
Copy link
Collaborator

@Hydrocharged Hydrocharged commented Apr 22, 2025

This implements the changes necessitated by the following fix:

We had a bug where calls to any DML statements that included an interpreted function would remove the DML's ability to actually affect the data. The GMS PR implements the core fix, and this PR implements the companion changes.

@Hydrocharged Hydrocharged requested a review from fulghum April 22, 2025 13:03
@Hydrocharged Hydrocharged mentioned this pull request Apr 22, 2025
Merged
@Hydrocharged
Copy link
Collaborator Author

Hydrocharged commented Apr 22, 2025

Tests will fail until GMS is merged since I'm not including the temporary go.mod changes to make merging a little more straightforward.

@github-actions
Copy link
Contributor

github-actions bot commented Apr 22, 2025 •
edited
Loading

Main PR
covering_index_scan_postgres 357.64/s 343.44/s -4.0%
index_join_postgres 153.88/s 152.80/s -0.8%
index_join_scan_postgres 186.29/s 184.52/s -1.0%
index_scan_postgres 12.46/s 12.38/s -0.7%
oltp_point_select 2624.59/s 2580.75/s -1.7%
oltp_read_only 1837.50/s 1802.95/s -1.9%
select_random_points 117.99/s 114.78/s -2.8%
select_random_ranges 132.45/s 130.27/s -1.7%
table_scan_postgres 11.60/s 11.89/s +2.5%
types_table_scan_postgres 5.48/s 5.49/s +0.1%

fulghum
fulghum approved these changes Apr 22, 2025
View reviewed changes
Copy link
Contributor

@fulghum fulghum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Hydrocharged Hydrocharged force-pushed the daylon/fixed-insert-interpreted branch from c263c7e to a323b28 Compare April 23, 2025 01:51
@Hydrocharged Hydrocharged enabled auto-merge April 23, 2025 01:52
@Hydrocharged Hydrocharged force-pushed the daylon/fixed-insert-interpreted branch from a323b28 to b44f7ce Compare April 23, 2025 01:53
@BrewTestBot BrewTestBot mentioned this pull request Apr 23, 2025
Merged
@github-actions
Copy link
Contributor

github-actions bot commented Apr 23, 2025

Main PR
Total 42090 42090
Successful 16361 16361
Failures 25729 25729
Partial Successes1 5530 5530
Main PR
Successful 38.8715% 38.8715%
Failures 61.1285% 61.1285%

Footnotes

  1. These are tests that we're marking as Successful, however they do not match the expected output in some way. This is due to small differences, such as different wording on the error messages, or the column names being incorrect while the data itself is correct. ↩

@Hydrocharged Hydrocharged merged commit 0de5bc2 into main Apr 23, 2025
14 checks passed
@Hydrocharged Hydrocharged deleted the daylon/fixed-insert-interpreted branch April 23, 2025 02:27
@BrewTestBot BrewTestBot mentioned this pull request May 2, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fulghum fulghum fulghum approved these changes

Assignees

No one assigned

Labels

contribution

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Hydrocharged @fulghum @coffeegoddd